Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gas mechanics phase 1 implementation. #1471

Merged
merged 53 commits into from
Sep 21, 2023

Conversation

StefanIliev545
Copy link
Contributor

@StefanIliev545 StefanIliev545 commented Aug 18, 2023

Why this change is needed

In order to pay for L1 fees and L2 computational costs, we need to introduce gas mechanics feature.

What changes were made as part of this PR

Support is added for depositing Ethereum. When transactions get put in a batch, funds are deducted to pay for rollup publishing costs. Withdrawals do not populate merkle proofs in the receipts, thus do not work with this PR. Base fee is also disabled.

PR checks pre-merging

Please indicate below by ticking the checkbox that you have read and performed the required
PR checks

  • PR checks reviewed and performed

@StefanIliev545 StefanIliev545 changed the title WIP Gas mechanics phase 1 implementation. Sep 5, 2023
@StefanIliev545 StefanIliev545 marked this pull request as ready for review September 5, 2023 11:21
Copy link
Collaborator

@tudor-malene tudor-malene left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

first round of superficial review.

go/common/gethencoding/geth_encoding.go Outdated Show resolved Hide resolved
go/common/headers.go Outdated Show resolved Hide resolved
go/common/headers.go Outdated Show resolved Hide resolved
go/enclave/components/batch_executor.go Outdated Show resolved Hide resolved
go/enclave/components/rollup_compression.go Outdated Show resolved Hide resolved
go/enclave/components/rollup_compression.go Show resolved Hide resolved
go/enclave/components/rollup_compression.go Outdated Show resolved Hide resolved
go/enclave/gas/gas.go Show resolved Hide resolved
return &gPool, nil
}

func CalculateL1GasUsed(data []byte, overhead *big.Int) *big.Int {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this needs a comment.
If I were to guess, this tries to estimate how much it will cost storing this tx to the l1.
If yes, I don't think it's working correctly.
Because the transaction will be compressed and encrypted.
I would say, it can only work by estimating. Taking the size, reducing it to 66% and then multiply by the cost of a non-zero byte

Copy link
Contributor Author

@StefanIliev545 StefanIliev545 Sep 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is how optimism does it. I'm not sure if taking the size directly and assuming it is all 1s is the correct approach to go, cause zero to ones ratio should somewhat be maintained after compression, right?

Nvm keep forgetting about encryption

go/enclave/gas/oracle.go Show resolved Hide resolved
Copy link
Collaborator

@tudor-malene tudor-malene left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

few more comments

ReceiptHash common.Hash `json:"receiptsRoot"`
Number *big.Int `json:"number"` // height of the batch
SequencerOrderNo *big.Int `json:"sequencerOrderNo"` // multiple batches can be created with the same height in case of L1 reorgs. The sequencer is responsible for including all of them in the rollups.
GasLimit uint64 `json:"gasLimit"`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our maximum gas limit per batch should be the one from ethereum/12.
our batch time =1s

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when the gas limit of a batch fills up, it should put all unexecuted transactions back in the mempool and return the batch

go/enclave/components/batch_executor.go Outdated Show resolved Hide resolved
}
accBalance := stateDB.GetBalance(*sender)

cost, err := executor.gasOracle.GetGasCostForTx(tx, block)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's call this EstimateL1StorageGasCost

go/enclave/components/batch_executor.go Show resolved Hide resolved
Copy link
Collaborator

@tudor-malene tudor-malene left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@StefanIliev545 StefanIliev545 merged commit c4864da into main Sep 21, 2023
3 checks passed
@StefanIliev545 StefanIliev545 deleted the sileiv/gas-mechanics-implementation branch September 21, 2023 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants